Skip to content

fix(observability): restore legacy dual-bucket Grafana RBAC sync - #2688

Merged
shikanime merged 1 commit into
mainfrom
fix/observability-dual-bucket
Sep 8, 2026
Merged

fix(observability): restore legacy dual-bucket Grafana RBAC sync#2688
shikanime merged 1 commit into
mainfrom
fix/observability-dual-bucket

Conversation

@shikanime

@shikanime shikanime commented Sep 7, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Issues liées

Issues numéro: Refs #2686

Quel est le comportement actuel ?

Depuis la migration du plugin observability vers server-nestjs (#2418), getListPerms (apps/server-nestjs/src/modules/observability/observability.utils.ts) range chaque utilisateur dans une seule paire de groupes Keycloak Grafana : prod si au moins un environnement prod existe, hors-prod sinon. Le plugin historique (console-plugin-observability) rangeait chaque utilisateur dans les deux paires. Comme reconcileGroupMembership est une sync diff-and-remove, le premier project.upsert sous server-nestjs a retiré tous les membres du groupe opposé — et Grafana associe ses rôles org à partir de ces groupes : perte d'accès déterministe.

Quel est le nouveau comportement ?

Bucketing par stage restauré (parité legacy, doc RBAC §3 : « Les deux peuvent coexister ») : un utilisateur apparaît dans la paire prod si un environnement prod existe et dans la paire hors-prod si un environnement non-prod existe. Les assertions du spec unitaire qui codefaient la sémantique mono-bucket sont corrigées, et un test de non-régression garantit que reconcileGroupMembership ne retire personne quand les deux stages existent. Un spec e2e (test/observability-sync.e2e-spec.ts) rejoue la chaîne complète — événement project.upsert → AppEventsService → ObservabilityService → getListPerms → Keycloak en mémoire — et vérifie l'état final des groupes, pas les appels.

Cette PR introduit-elle un breaking change ?

Non. Le correctif restaure le comportement documenté avant migration. Remédiation des instances affectées : rejouer project.upsert par projet concerné — la sync ré-ajoute les adhésions détruites (idempotent).

Autres informations

@github-actions github-actions Bot added the built label Sep 7, 2026
@shikanime
shikanime force-pushed the fix/observability-dual-bucket branch from b6fbf16 to ca0cc82 Compare September 8, 2026 09:06
@shikanime shikanime mentioned this pull request Sep 8, 2026
Closed
5 tasks
@shikanime
shikanime force-pushed the fix/observability-dual-bucket branch from ca0cc82 to b67e78b Compare September 8, 2026 09:11
@shikanime
shikanime marked this pull request as ready for review September 8, 2026 09:17
@shikanime
shikanime requested a review from a team as a code owner September 8, 2026 09:17
@shikanime shikanime added this to the 9.25.1 milestone Sep 8, 2026
@shikanime shikanime added the preview Deploy preview app with Argo-cd label Sep 8, 2026
@shikanime shikanime self-assigned this Sep 8, 2026
@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

🤖 Hey !

A preview of the application is available at : https://console-pr-2688.dso.cpin-hp.numerique-interieur.fr

Please be patient, deployment may take a few minutes.

@shikanime
shikanime force-pushed the fix/observability-dual-bucket branch 3 times, most recently from d83efec to 1950b3f Compare September 8, 2026 10:14
@shikanime
shikanime marked this pull request as draft September 8, 2026 10:51
@shikanime
shikanime force-pushed the fix/observability-dual-bucket branch from 1950b3f to 08f3ece Compare September 8, 2026 11:47
shikanime
shikanime commented Sep 8, 2026
View reviewed changes

@shikanime shikanime left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict : Approuvé (auto-revue — les deux ajustements ci-dessous sont appliqués dans le commit qui suit)

Le remplissage par bucket restaure fidèlement la sémantique legacy, les specs unitaires verrouillent la parité dans les deux sens, et le spec e2e rejoue la chaîne complète contre Keycloak réel. L'extraction de addUserToBucket améliore nettement la lisibilité de getListPerms sans changer le comportement.

Comment thread apps/server-nestjs/src/modules/observability/observability.utils.ts Outdated
Comment thread apps/server-nestjs/src/modules/observability/observability.service.spec.ts Outdated
Comment thread apps/server-nestjs/test/observability-sync.e2e-spec.ts
Comment thread apps/server-nestjs/src/modules/observability/observability.utils.ts
@shikanime
shikanime force-pushed the fix/observability-dual-bucket branch 2 times, most recently from c773596 to 1997bf4 Compare September 8, 2026 12:09
@shikanime
shikanime marked this pull request as ready for review September 8, 2026 12:11
@shikanime
shikanime force-pushed the fix/observability-dual-bucket branch from 1997bf4 to 4f30572 Compare September 8, 2026 12:15
@shikanime

shikanime commented Sep 8, 2026

Copy link
Copy Markdown
Member Author
image

@shikanime shikanime added the bug Something isn't working label Sep 8, 2026
@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

🤖 Hey !

A preview of the application is available at : https://console-pr-2688.dso.cpin-hp.numerique-interieur.fr

Please be patient, deployment may take a few minutes.

@shikanime
shikanime enabled auto-merge September 8, 2026 12:50
@shikanime shikanime modified the milestones: 9.25.1, 9.25.2 Sep 8, 2026
@shikanime
shikanime marked this pull request as draft September 8, 2026 13:06
auto-merge was automatically disabled September 8, 2026 13:06

Pull request was converted to draft

@shikanime
fix(observability): restore legacy dual-bucket Grafana RBAC sync
5e1c760 
getListPerms filled only one stage bucket pair (prod if any prod env
exists, else hors-prod), where the legacy plugin filled both. Since
reconcileGroupMembership removes members absent from the desired list,
the first project.upsert under server-nestjs wiped every membership of
the opposite pair — and Grafana maps org roles from exactly those
Keycloak groups, so users deterministically lost access.

Restore per-stage bucketing: a user lands in prod when a prod env
exists AND in hors-prod when a real (named) non-prod stage exists
(RBAC doc §3: 'Les deux peuvent coexister'). Fix the spec assertions
that codified the single-bucket contract and add a reconcile
regression guard.

Extends the observability service spec with an E2E-gated suite that
drives the production event chain (event bus -> AppEventsService ->
ObservabilityService -> getListPerms -> real Keycloak from the dev
stack) and asserts final group membership against real Postgres +
Keycloak, regression-locking the dual-bucket fill end to end.

Refs #2686
Signed-off-by: William Phetsinorath <william.phetsinorath-open@interieur.gouv.fr>
Change-Id: Ic8c8e2f352d164737f7a8ea3635e3d1b6a6a6964
@shikanime
shikanime force-pushed the fix/observability-dual-bucket branch from 4f30572 to 5e1c760 Compare September 8, 2026 13:12
@shikanime
shikanime marked this pull request as ready for review September 8, 2026 13:13
@cloud-pi-native-sonarqube

cloud-pi-native-sonarqube Bot commented Sep 8, 2026

Copy link
Copy Markdown

Passed Quality Gate passed

Issues

Measures

Project ID: cloud-pi-native_console_AYsa46O31ebDtzs-pYyZ

View in SonarQube

@shikanime
shikanime enabled auto-merge September 8, 2026 13:20
@shikanime shikanime mentioned this pull request Sep 8, 2026
Draft
shikanime added a commit that referenced this pull request Sep 8, 2026
@shikanime
refactor(server-nestjs): extract e2e spec gates to per-file utils
f39a964 
Each e2e spec keeps only its describe body; the E2E gate, its
describe.runIf alias (and, for observability, the shared Grafana
subgroup constants) move to a sibling <name>.utils.ts. The vitest
include globs only match *.e2e-spec.ts, so utils files stay out of
both unit and gated runs.

Stacked on fix/observability-dual-bucket (PR #2688).

Signed-off-by: William Phetsinorath <william.phetsinorath-open@interieur.gouv.fr>
Change-Id: I602e5529ac2cea360ce6025d4f06e0b9efe359e9
@shikanime
shikanime added this pull request to the merge queue Sep 8, 2026
Merged via the queue into main with commit 2ad58c1 Sep 8, 2026
62 checks passed
@shikanime
shikanime deleted the fix/observability-dual-bucket branch September 8, 2026 16:14
shikanime added a commit that referenced this pull request Sep 9, 2026
@shikanime
refactor(server-nestjs): extract e2e spec gates to per-file utils
a451361 
Each e2e spec keeps only its describe body; the E2E gate, its
describe.runIf alias (and, for observability, the shared Grafana
subgroup constants) move to a sibling <name>.utils.ts. The vitest
include globs only match *.e2e-spec.ts, so utils files stay out of
both unit and gated runs.

Stacked on fix/observability-dual-bucket (PR #2688).

Signed-off-by: William Phetsinorath <william.phetsinorath-open@interieur.gouv.fr>
Change-Id: I602e5529ac2cea360ce6025d4f06e0b9efe359e9
shikanime added a commit that referenced this pull request Sep 9, 2026
@shikanime
refactor(server-nestjs): extract e2e spec gates to per-file utils
975d543 
Each e2e spec keeps only its describe body; the E2E gate, its
describe.runIf alias (and, for observability, the shared Grafana
subgroup constants) move to a sibling <name>.utils.ts. The vitest
include globs only match *.e2e-spec.ts, so utils files stay out of
both unit and gated runs.

Stacked on fix/observability-dual-bucket (PR #2688).

Signed-off-by: William Phetsinorath <william.phetsinorath-open@interieur.gouv.fr>
Change-Id: I602e5529ac2cea360ce6025d4f06e0b9efe359e9
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@StephaneTrebel StephaneTrebel StephaneTrebel approved these changes

Assignees

@shikanime shikanime

Labels

bug Something isn't working built preview Deploy preview app with Argo-cd

Projects

None yet

Milestone

9.25.2

Development

Successfully merging this pull request may close these issues.

2 participants

@shikanime @StephaneTrebel